Skip to content

Update TestContainers and change docker depenency to moby#1513

Merged
nodece merged 6 commits into
apache:masterfrom
kellyfj:move-docker-dependency-to-moby
Jun 24, 2026
Merged

Update TestContainers and change docker depenency to moby#1513
nodece merged 6 commits into
apache:masterfrom
kellyfj:move-docker-dependency-to-moby

Conversation

@kellyfj

@kellyfj kellyfj commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation

Update dependencies - esp for docker dependency that has this CVE https://nvd.nist.gov/vuln/detail/CVE-2026-34040

Modifications

  1. Updated Golang from 1.24.0 to 1.25.0 (required by testcontainers library https://github.com/testcontainers/testcontainers-go/blob/8360f719408c23a4f3b731be25a94b267b624a28/AI.md#go-version)
  2. Updated testcontainers to latest (which stops using docker dependency and uses moby)
  3. Changed consumer_zero_queue_test.go to use new moby dependencies

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): YES

Documentation

  • Does this pull request introduce a new feature? NO

@kellyfj kellyfj marked this pull request as ready for review June 18, 2026 13:18
@kellyfj

kellyfj commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Can someone please advise on this failing integration test?
Thanks

Tests failed
--- FAIL: TestSendBufferRetainWhenConnectionStuck (0.15s)
doing stop standalone ...

Also looks like I need to upgrade the linter with associated new linter requirements. Will seek to do that in a separate PR.

UPDATE: I created #1515 to keep the golang update (with linter and related code cleanup) as a separate PR for clean git commits and limit the "blast radius" of my change. So I think this PR will depend on that one (I will rebase once 1515 is merged) if that makes sense. Thanks!

@kellyfj kellyfj mentioned this pull request Jun 20, 2026
Merged
@crossoverJie crossoverJie requested a review from Copilot June 22, 2026 02:48
Copilot AI reviewed Jun 22, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the project’s Go and container-testing dependencies to address a Docker-related CVE risk by moving TestContainers usage away from github.com/docker/docker and onto Moby libraries, and adapts affected tests accordingly.

Changes:

  • Bumped module Go version to 1.25.0 and refreshed a number of dependencies (notably testcontainers-go).
  • Removed direct dependency on github.com/docker/docker and introduced github.com/moby/moby/api for container/network types.
  • Updated consumer_zero_queue_test.go to use Moby’s network/port binding types.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
pulsar/consumer_zero_queue_test.go Switches port binding setup from Docker/nat types to Moby network types for TestContainers usage.
go.mod Updates Go version and dependency set (TestContainers bump; Docker -> Moby API).
go.sum Synchronizes module checksums with the updated dependency graph.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pulsar/consumer_zero_queue_test.go Outdated
Comment thread pulsar/consumer_zero_queue_test.go Outdated
nodece and others added 2 commits June 22, 2026 11:35
@nodece
Apply suggestions from code review
90fda31 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@kellyfj
Fix producer_test race condition
0bec651
@kellyfj

kellyfj commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

The issue with TestSendBufferRetainWhenConnectionStuck is a data race in the mockConn.WriteData() method. The test's assert.Eventually is reading from conn.buffers without holding the lock, while the producer goroutine is writing to it with the lock.

The fix is to protect both the write and the read with the mutex.

@nodece nodece merged commit d0dd423 into apache:master Jun 24, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@nodece nodece nodece approved these changes
@crossoverJie crossoverJie Awaiting requested review from crossoverJie
@freeznet freeznet Awaiting requested review from freeznet
@Demogorgon314 Demogorgon314 Awaiting requested review from Demogorgon314
@RobertIndie RobertIndie Awaiting requested review from RobertIndie

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kellyfj @nodece @crossoverJie